Skip to content

feat(epp): order all requestcontrol hooks by data-dependency DAG#15

Open
chethanuk wants to merge 3 commits into
mainfrom
fix/issue-2040-feat-order-all-plugin-hook-lists
Open

feat(epp): order all requestcontrol hooks by data-dependency DAG#15
chethanuk wants to merge 3 commits into
mainfrom
fix/issue-2040-feat-order-all-plugin-hook-lists

Conversation

@chethanuk

@chethanuk chethanuk commented Jul 17, 2026

Copy link
Copy Markdown
Owner

User description

What type of PR is this?
/kind bug
/kind feature

What this PR does / why we need it:

ValidateAndOrderDataDependencies already built a topological order of requestcontrol plugins, but only DataProducer plugins were reordered. The other five hook lists (PreRequest, RequestHeader, Admission, ResponseReceived, ResponseStreaming) kept the map-iteration order from GetAllPlugins, so a consumer could run before its producer after a restart (the llm-d#1856 class of bug).

This PR:

  1. Makes TopologicalSort deterministic with a largest-ready Kahn tie-break (so the existing reverse yields ascending name ties).
  2. Replaces OrderDataProducerPlugins with OrderPlugins, which reorders all six requestcontrol hook lists by the DAG rank while retaining unranked plugins (stable append by name).
  3. Documents that Consumes/Produces govern every requestcontrol extension point.

Not a duplicate of llm-d#1421 / PR llm-d#1876: that work ordered plugin instantiation in the config loader; this orders hook execution in requestcontrol.

Which issue(s) this PR fixes:
Fixes llm-d#2040

How to test:

  • make test-filter PATTERN=TestConfig_OrderPlugins_AllHookLists TYPE=epp epp_TEST_PACKAGES='./pkg/epp/requestcontrol/' — 6 subtests PASS (all six lists ordered; unranked retained)
  • make test-filter PATTERN=TestTopologicalSort_Deterministic TYPE=epp epp_TEST_PACKAGES='./pkg/epp/util/' — PASS
  • make test-filter PATTERN=TestDAGAndTopologicalOrder TYPE=epp epp_TEST_PACKAGES='./pkg/epp/datalayer/' — PASS
  • make test-filter PATTERN='TestPluginsWithDependencies|TestFilterExecutionOrderFromYAML' TYPE=epp epp_TEST_PACKAGES='./pkg/epp/config/loader/' — PASS (2nd TopologicalSort caller / instantiation order)
  • make lint — 0 issues
  • make build — epp + sidecar
  • make test-unit-epp (excl. pre-existing pkg/kvcache/kvblock full-suite load flake; package green in isolation) + make test-unit-sidecar

Release note (write NONE if no user-facing change):

Requestcontrol plugins on every extension point (not only DataProducer) now run in data-dependency DAG order: producers before consumers, with a deterministic tie-break. Removes restart-dependent PreRequest/header/response hook ordering.

CodeAnt-AI Description

Order every request hook by data dependencies

What Changed

  • Plugins that depend on other data now run after their producers across all request hook paths, not just the main data-producing path.
  • Unranked plugins are still kept, and they now appear in a stable name order instead of changing between runs.
  • Dependency ordering is now repeatable, so the same plugin graph produces the same execution order every time.
  • Added coverage for the ordering rules across all hook lists and for edge cases like empty configs and missing names.

Impact

✅ Fewer cases where a consumer runs before its producer
✅ Stable hook order after restart
✅ More predictable request processing

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Summary by CodeRabbit

  • Improvements

    • Plugin execution now consistently follows declared data dependencies across request, admission, and response processing stages.
    • Plugin ordering is deterministic, including plugins without declared dependencies.
    • Dependency-based ordering now handles ties consistently for predictable results.
  • Bug Fixes

    • Corrected configuration ordering so all applicable plugin types follow the computed dependency order.

ValidateAndOrderDataDependencies already produced a topological order, but
only DataProducer plugins were reordered. Other extension points kept the
map-iteration order from GetAllPlugins, so PreRequest (and related hooks)
could run consumers before producers across restarts (llm-d#1856 class of bug).

Make TopologicalSort deterministic via a largest-ready tie-break (so the
existing reverse yields ascending name ties), apply that order to all six
requestcontrol hook lists while retaining unranked plugins, and document
that Consumes/Produces govern every requestcontrol extension point.

Signed-off-by: ChethanUK <chethanuk@outlook.com>
@codeant-ai

codeant-ai Bot commented Jul 17, 2026

Copy link
Copy Markdown

CodeAnt AI is reviewing your PR.

@codeant-ai

codeant-ai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@chethanuk, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 22 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1af0c45c-3f7e-4bc3-9ee9-ff4c0e613f3a

📥 Commits

Reviewing files that changed from the base of the PR and between c965fd1 and 4e93151.

📒 Files selected for processing (2)
  • pkg/epp/framework/interface/plugin/plugins.go
  • pkg/epp/requestcontrol/request_control_config_test.go
📝 Walkthrough

Walkthrough

The request-control configuration now orders all hook plugins from the dependency DAG, with deterministic handling for ties and unranked plugins. Topological sorting receives deterministic tie-breaking, and tests cover hook-list ordering, edge cases, dependency shapes, and cycle detection.

Changes

Request-control plugin ordering

Layer / File(s) Summary
Deterministic dependency graph ordering
pkg/epp/util/utils.go, pkg/epp/util/utils_test.go
TopologicalSort deterministically resolves ready-node ties, with tests covering dependency graphs and cycle errors.
All-hook plugin ordering
pkg/epp/requestcontrol/request_control_config.go, pkg/epp/requestcontrol/request_control_config_test.go, pkg/epp/framework/interface/plugin/plugins.go
OrderPlugins applies DAG ranks across request-control hook lists, sorts unranked plugins by typed name, and tests ranked, unranked, missing, and empty inputs.
Configuration phase integration
cmd/epp/runner/runner.go
Configuration phase two invokes OrderPlugins with the computed dependency order.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: shmuelk

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: applying the data-dependency DAG to all requestcontrol hooks.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-2040-feat-order-all-plugin-hook-lists

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Jul 17, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the plugin ordering mechanism to ensure deterministic execution across all request control extension points, addressing potential order-dependency issues. It introduces a unified OrderPlugins method, refactors the topological sort utility to break ties deterministically, and adds comprehensive unit tests. The reviewer provided valuable feedback on performance optimizations: first, to avoid sorting the entire queue in Kahn's algorithm by finding the maximum element in linear time; and second, to cache plugin names and ranks prior to sorting in orderByName to eliminate redundant string formatting and map lookups.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@codeant-ai

codeant-ai Bot commented Jul 17, 2026

Copy link
Copy Markdown

CodeAnt AI finished reviewing your PR.

The whole point of ordering every hook list by the data-dependency DAG is
that the result must not depend on Go's randomized map iteration in
GetAllPlugins. The existing tests fed a single input order, so a regression
that reintroduced input-order sensitivity could pass. Drive several fixed
permutations of the same plugin set and assert every hook list comes out
identical.

Signed-off-by: ChethanUK <chethanuk@outlook.com>
Ordering every requestcontrol hook list by the data-dependency DAG makes
execution deterministic, which is the goal. The producer-before-consumer
direction is right for the request-path hooks, but for ResponseHeader and
ResponseBody a producer may release the state a consumer reads, so the
correct direction there is not established. Document it as unsettled rather
than asserting producer-first is canonical for response hooks.

Signed-off-by: ChethanUK <chethanuk@outlook.com>
Repository owner deleted a comment from gemini-code-assist Bot Jul 19, 2026
Repository owner deleted a comment from gemini-code-assist Bot Jul 19, 2026
Repository owner deleted a comment from codeant-ai Bot Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] Order all plugin hook lists by the data-dependency DAG, not just Produce

1 participant